Review requested:

• @nodejs/single-executable
• @nodejs/test_runner

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @avivkeller.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

Nice! This is a great addition. Since it's such a large PR, this will take me some time to review. Will try to tackle it over the next week.

Left an initial review, but like @Ethan-Arrowood said, it'll take time for a more in depth look

It's nice to see some momentum in this area, though from a first glance it seems the design has largely overlooked the feedback from real world use cases collected 4 years ago: https://github.com/nodejs/single-executable/blob/main/docs/virtual-file-system-requirements.md - I think it's worth checking that the API satisfies the constraints that users of this feature have provided, to not waste the work that have been done by prior contributors to gather them, or having to reinvent it later (possibly in a breaking manner) to satisfy these requirements from real world use cases.

Codecov Report

❌ Patch coverage is 87.58461% with 752 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.69%. Comparing base (b864049) to head (164ba61).
⚠️ Report is 53 commits behind head on main.

@@            Coverage Diff             @@
##             main   #61478      +/-   ##
==========================================
- Coverage   89.74%   89.69%   -0.06%     
==========================================
  Files         675      690      +15     
  Lines      204601   210789    +6188     
  Branches    39325    40177     +852     
==========================================
+ Hits       183616   189057    +5441     
- Misses      13273    13984     +711     
- Partials     7712     7748      +36     

And why not something like OPFS aka whatwg/fs?

const rootHandle = await navigator.storage.getDirectory()
await rootHandle.getFileHandle('config.json', { create: true })
fs.mount('/app', rootHandle) // to make it work with fs
fs.readFileSync('/app/config.json')

OR

const rootHandle = await navigator.storage.getDirectory()
await rootHandle.getFileHandle('config.json', { create: true })

fs.readFileSync('sandbox:/config.json')

fs.createVirtual seems like something like a competing specification

And why not something like OPFS aka whatwg/fs?

I generally prefer not to interleave with WHATWG specs as much as possible for core functionality (e.g., SEA). In my experience, they tend to perform poorly on our codebase and remove a few degrees of flexibility. (I also don't find much fun in working on them, and I'm way less interested in contributing to that.)

On an implementation side, the core functionality of this feature will be identical (technically, it's missing writes that OPFS supports), as we would need to impact all our internal fs methods anyway.

If this lands, we can certainly iterate on a WHATWG-compatible API for this, but I would not add this to this PR.

Small prior art: https://github.com/juliangruber/subfs

I also worked on this a bit on the side recently: Qard@73b8fc6

That is very much in chaotic ideation stage with a bunch of LLM assistance to try some different ideas, but the broader concept I was aiming for was to have a VirtualFileSystem type which would actually implement the entire API surface of the fs module, accepting a Provider type to delegate the internals of the whole cluster of file system types to a singular class managing the entire cluster of fs-related types such that the fs module could actually just be fully converted to:

module.exports = new VirtualFileSystem(new LocalProvider())

I intended for it to be extensible for a bunch of different interesting scenarios, so there's also an S3 provider and a zip file provider there, mainly just to validate that the model can be applied to other varieties of storage systems effectively.

Keep in mind, like I said, the current state is very much just ideation in a branch I pushed up just now to share, but I think there are concepts for extensibility in there that we could consider to enable a whole ecosystem of flexible storage providers. 🙂

Personally, I would hope for something which could provide both read and write access through an abstraction with swappable backends of some variety, this way we could pass around these virtualized file systems like objects and let an ecosystem grow around accepting any generalized virtual file system for its storage backing. I think it'd be very nice for a lot of use cases like file uploads or archive management to be able to just treat them like any other readable and writable file system.

Personally, I would hope for something which could provide both read and write access through an abstraction with swappable backends of some variety, this way we could pass around these virtualized file systems like objects and let an ecosystem grow around accepting any generalized virtual file system for its storage backing. I think it'd be very nice for a lot of use cases like file uploads or archive management to be able to just treat them like any other readable and writable file system.

just a bit off topic... but this reminds me of why i created this feature request:
Blob.from() for creating virtual Blobs with custom backing storage

Would not lie, it would be cool if NodeJS also provided some type of static Blob.from function to create virtual lazy blobs. could live on fs.blobFrom for now...

example that would only work in NodeJS (based on how it works internally)

const size = 26

const blobPart = BlobFrom({
  size,
  stream (start, end) {
    // can either be sync or async (that resolves to a ReadableStream)
    // return new Response('abcdefghijklmnopqrstuvwxyz'.slice(start, end)).body
    // return new Blob(['abcdefghijklmnopqrstuvwxyz'.slice(start, end)]).stream()
    
    return fetch('https://httpbin.dev/range/' + size, {
      headers: {
        range: `bytes=${start}-${end - 1}`
      }
    }).then(r => r.body)
  }
})

blobPart.text().then(text => {
  console.log('a-z', text)
})

blobPart.slice(-3).text().then(text => {
  console.log('x-z', text)
})

const a = blobPart.slice(0, 6)
a.text().then(text => {
  console.log('a-f', text)
})

const b = a.slice(2, 4)
b.text().then(text => {
  console.log('c-d', text)
})

x-z xyz
a-z abcdefghijklmnopqrstuvwxyz
a-f abcdef
c-d cd

An actual working PoC

(I would not rely on this unless it became officially supported by nodejs core - this is a hack)

const blob = new Blob()
const symbols = Object.getOwnPropertySymbols(blob)
const blobSymbol = symbols.map(s => [s.description, s])
const symbolMap = Object.fromEntries(blobSymbol)
const {
  kHandle,
  kLength,
} = symbolMap

function BlobFrom ({ size, stream }) {
  const blob = new Blob()
  if (size === 0) return blob

  blob[kLength] = size
  blob[kHandle] = {
    span: [0, size],

    getReader () {
      const [start, end] = this.span
      if (start === end) {
        return { pull: cb => cb(0) }
      }

      let reader

      return {
        async pull (cb) {
          reader ??= (await stream(start, end)).getReader()
          const {done, value} = await reader.read()
          cb(done ^ 1, value)
        }
      }
    },

    slice (start, end) {
      const [baseStart] = this.span

      return {
        span: [baseStart + start, baseStart + end],
        getReader: this.getReader,
        slice: this.slice,
      }
    }
  }

  return blob
}

currently problematic to do: new Blob([a, b]), new File([blobPart], 'alphabet.txt', { type: 'text/plain' })

also need to handle properly clone, serialize & deserialize, if this where to be sent of to another worker - then i would transfer a MessageChannel where the worker thread asks main frame to hand back a transferable ReadableStream when it needs to read something.

but there are probably better ways to handle this internally in core with piping data directly to and from different destinations without having to touch the js runtime? - if only getReader could return the reader directly instead of needing to read from the ReadableStream using js?

CI: https://ci.nodejs.org/job/node-test-pull-request/71329/

There are roughly ~70 windows failures. Guess I have some
windows work to do!

I feel like there should be a mention of this API somewhere in the fs.md but I'm not sure where/how to do it.

I feel like there should be a mention of this API somewhere in the fs.md but I'm not sure where/how to do it.

I think that it would be useful to have some sort of "supported/not supported in vfs" annotation to each API in fs.md? It would lower the mental burden for users when they need to look up the docs - I suspect people are more likely to try to confirm whether something works/doesn't work in vfs than trying to directly use vfs APIs and only vfs, not switching to real fs at any point.

I'm not a huge fan of the mount(...) implicitly hijacking raw fs behaviour rather than an explicitly constructed overlay. The idea I was going for with my implementation was to mirror the API so dependency injection patterns could be used to pass in an fs-conforming interface and it would use whatever is received, but anything using the fs module directly would just go straight to the actual disk. I'm not going to hard-block on that right now, but I'm a bit concerned about the security implications of allowing any code anywhere to hijack entire directories within the filesystem. I can imagine a bunch of possible attacks related to loading different files than expected in various scenarios. 🤔

My thinking was something like:

function someApi(fs: FsLike) {
  fs.readFile(...)
}

// Works with just the base fs provider itself.
someApi(require('fs'))

// Also works with any provider that conforms to the FsLike contract.
// Because mounting exists on the provider rather than the actual vfs
// additional mounts can't be set from elsewhere without access to the
// original provider.
const mockOverlay = new OverlayProvider(require('fs'))
mockOverlay.mount('/mount', new MockProvider(...))

someApi(new VirtualFileSystem(mockOverlay))

That hijack is what makes it viable for the ecosystem without changing their current APIs.

Perhaps, but it's also somewhat risky. Perhaps we need to think about how we can apply the permission model controls to that to prevent unintended tampering with the global fs?